fix: send local state when ICE connects instead of at participant-add time#6086
fix: send local state when ICE connects instead of at participant-add time#6086alauzon wants to merge 7 commits intonextcloud:masterfrom
Conversation
|
Thank you for contributing @alauzon Please also solve the DCO warning be following https://github.com/nextcloud/talk-android/pull/6086/checks?check_run_id=71734417320 |
alauzon
force-pushed
the
fix/local-state-not-sent-after-ice-connected
branch
2 times, most recently
from
046eef9 to
b05d8f8
Compare
… time When a participant was added, LocalStateBroadcasterNoMcu.handleCallParticipantAdded was called with a snapshot of the current ParticipantUiState. At that moment the PeerConnectionWrapper did not yet exist, so the data channel was unavailable and all state messages (audioOn/videoOn/speaking) were silently dropped. Subsequent ICE transitions were never observed, so the remote participant never learned the local audio/video state. Fix: LocalStateBroadcasterNoMcu now accepts a live StateFlow<ParticipantUiState> and collects it internally via IceConnectionStateObserver. State is sent on every false→true transition of isConnected, i.e. exactly when the data channel becomes ready. CallActivity passes the live StateFlow instead of the snapshot value. A non-abstract default overload is added to LocalStateBroadcaster so that LocalStateBroadcasterMcu continues to work unchanged. Tests: - LocalStateBroadcasterNoMcuTest: drives state via MutableStateFlow; fails without the fix because updates after handleCallParticipantAdded are never seen - CallParticipantStateBroadcastIntegrationTest: full pipeline test through CallViewModel → ParticipantHandler → LocalStateBroadcasterNoMcu with a mocked PeerConnectionWrapper driving ICE transitions - CallActivityAddParticipantTest: Robolectric test verifying that CallActivity passes the live StateFlow object (not a snapshot) to the broadcaster Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Alain Lauzon <alauzon@alainlauzon.com>
alauzon
force-pushed
the
fix/local-state-not-sent-after-ice-connected
branch
from
b05d8f8 to
2c5d311
Compare
Done |
|
Please excuse the current delays in pull request reviews. |
Done |
2 participants
Fixes #6037
Problem
When a remote participant was added to a call (no MCU),
LocalStateBroadcasterNoMcu.handleCallParticipantAddedwas called with a snapshot of the currentParticipantUiState. At that moment thePeerConnectionWrapperdid not yet exist, so the data channel was unavailable and all state messages (audioOn/videoOn/speaking) were silently dropped.After
setPeerConnection()was called and ICE negotiation completed (isConnectedbecametrue), nobody calledhandleCallParticipantAddedagain — so the remote participant never learned the local audio/video state.Consequence: the remote participant saw a grey/unknown icon for local audio and video until the local user manually toggled their mic or camera. Guests joining with audio-only could appear as "connecting" with no audio permanently.
Fix
LocalStateBroadcasterNoMcunow accepts a liveStateFlow<ParticipantUiState>and collects it internally via anIceConnectionStateObservercoroutine. State is sent on everyfalse → truetransition ofisConnected, i.e. exactly when the data channel becomes ready.CallActivity.addCallParticipantnow passes the liveStateFlow(one line) instead of the snapshot.value.A non-abstract default overload is added to
LocalStateBroadcasterso thatLocalStateBroadcasterMcucontinues to work unchanged.Tests
Three test classes are added or rewritten:
LocalStateBroadcasterNoMcuTest— drivesisConnectedviaMutableStateFlow; fails without the fix because updates afterhandleCallParticipantAddedare never observedCallParticipantStateBroadcastIntegrationTest— full pipeline test throughCallViewModel → ParticipantHandler → LocalStateBroadcasterNoMcuwith a mockedPeerConnectionWrapperdriving ICE transitionsCallActivityAddParticipantTest— Robolectric test verifying thatCallActivitypasses the liveStateFlowobject (not a snapshot) to the broadcasterAll tests were validated to fail without the fix and pass with it.